Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(RHINENG-7083): Temporarily ignore 403 when checking hosts count #2138

Merged
merged 1 commit into from
Jan 15, 2024
Merged

fix(RHINENG-7083): Temporarily ignore 403 when checking hosts count #2138

merged 1 commit into from
Jan 15, 2024

Conversation

gkarat
Copy link
Contributor

@gkarat gkarat commented Jan 15, 2024 •
edited
Loading

Fixes https://issues.redhat.com/browse/RHINENG-7083.

This fixes the issue with users having no inventory:hosts:read permissions trying to access the /inventory page. Now, while checking for hosts count, if we get 403, we should proceed and set the loading flag to false. The permissions are further handled by the /inventory route again so it's safe.

⚠️ This is an intermediate (workaround) fix because of the time pressure (but the proper refactor PR will be open soon).

How to test

  1. Have no 'inventory:hosts:read' permissions for your account
  2. Navigate to /inventory
  3. If you have at least one host (conventional/immutable), you should see the table rendered
  4. If you don't have any hosts, you should see the "zero stage"

@gkarat gkarat added the bug Something isn't working label Jan 15, 2024
@gkarat gkarat self-assigned this Jan 15, 2024
@gkarat gkarat requested a review from a team as a code owner January 15, 2024 12:31
@gkarat
Copy link
Contributor Author

gkarat commented Jan 15, 2024

/retest

@codecov-commenter
Copy link

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (c0f7e69) 57.16% compared to head (3329992) 57.14%.

Files Patch % Lines
src/Routes.js 40.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2138      +/-   ##
==========================================
- Coverage   57.16%   57.14%   -0.02%     
==========================================
  Files         194      194              
  Lines        6245     6247       +2     
  Branches     1742     1743       +1     
==========================================
  Hits         3570     3570              
- Misses       2675     2677       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adonispuente
adonispuente approved these changes Jan 15, 2024
View reviewed changes
Copy link
Contributor

@adonispuente adonispuente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seems to be working as intended.

@gkarat gkarat merged commit f2f6953 into RedHatInsights:master Jan 15, 2024
1 of 2 checks passed
@gkarat gkarat deleted the rhineng-7083 branch January 15, 2024 16:43
@gkarat
Copy link
Contributor Author

gkarat commented Jan 15, 2024

🎉 This PR is included in version 1.64.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@gkarat gkarat mentioned this pull request Apr 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adonispuente adonispuente adonispuente approved these changes

Assignees

@gkarat gkarat

Labels
bug Something isn't working released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gkarat @codecov-commenter @adonispuente